Fix deleting newly placed components with Backspace key #771
Conversation
|
Your Render PR Server URL is https://toolpad-pr-771.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-cbpv8gla499d2t4ac8c0. |
oliviertassinari
requested a deployment
to
fix-deleting-newly-placed-component - toolpad-db PR #771
Render
Abandoned
Signed-off-by: Pedro Ferreira <[email protected]>
| const overlayElement = overlayRef.current; | ||
| if (overlayElement) { | ||
| overlayElement.focus(); | ||
| } |
There was a problem hiding this comment.
Do we need the defensive check here? I would assume React guarantees the ref is defined, hence:
| const overlayElement = overlayRef.current; | |
| if (overlayElement) { | |
| overlayElement.focus(); | |
| } | |
| overlayRef.current!.focus(); |
would be better, so we catch regressions (e.g. the ref is no longer bound to a DOM element) at runtime instead of going silent.
There was a problem hiding this comment.
In some other places we do:
| const overlayElement = overlayRef.current; | |
| if (overlayElement) { | |
| overlayElement.focus(); | |
| } | |
| invariant(overlayRef.current, 'Ref not bound'); | |
| overlayRef.current.focus(); |
This communicates the intent a bit clearer and avoids usage of !.
Finding a few more occurrences using regex search in vscode if \([^.]+\.current\b
There was a problem hiding this comment.
good points, thanks! i used an invariant check here and also in a couple other places where it seemed right to use it
|
Your Render PR Server URL is https://toolpad-pr-771.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-cbtp50pgp3jh0klggbg0. |
oliviertassinari
requested a deployment
to
fix-deleting-newly-placed-component - toolpad-db PR #771
Render
Abandoned
Fixes issue where it was not possible to delete a component from the editor with the "Backspace" key just after placing it, without unselecting and then selecting it again.
My solution to this issue was to refocus on the overlay element that catches
keydownevents after placing a new element, because dragging it from outside the editor was moving the focus outside of it.Closes #752